First version of the X-ARAPUCAs decoder (V1740B digitizers) - #552
Conversation
… Fragments and add the needed parameters to the xarapucadecoder.fcl file.
… fragment before the fragment is being added to the fragments vector to be processed
…ta from multiple boards
…nd the board channel
…s decoder (V1740B digitizers).
…1740B digitizers binary decoding
There was a problem hiding this comment.
Hi Alicia -- sorry to be slow here! I didn't check all the logic of unpacking the raw waveforms -- I assume that's what the cout statements are there and also the two kinds of output, so you can test this decoder on data and tell whether it is doing its job. There are some things that could be optimized, such as reading the fragments straight out of the art event memory instead of copying them to a locally-owned vector called fragments, but as it goes out of scope at the end of the module, I won't fuss about it for getting this deployed. Copying data is actually a very quick operation on CPUs these days, but just allocating the memory, even temporarily, may raise the memory usage of a job. Of course if other modules use even more memory temporarily, this may not contribute to the high-water mark of a job. About how much data are there from X-ARAPUCAs per event? You can use product_sizes_dumper to find out.
One issue that may be bigger is the fact that it makes two copies of all the output. It makes OpDetWaveforms for the artROOT output file, but also TH1Ds for each waveform, saved into the TFileService output file (which may be named something other than decoder_hist.root, depending on the fcl config). Can you add a fcl parameter to steer the latter? In large data production campaigns, people probably won't look at every opdet waveform in the histogram files. So I'd imagine for production, we'd want to disable writing those TH1Ds anyway. But for debugging, you may want some of them, possibly not all. So a limit on how many you might want to save might be a good fcl parameter, as a signed integer. zero saves none, -1 saves them all, and n saves the first n of them. You can default that to -1, but I'll bet the production team would prefer 0.
| // - A decoder_hist.root file gathering a waveform histograms. | ||
| std::stringstream hist_name(""); | ||
| hist_name << "Event " << fevent_counter << " CH " << ch << " [frag " << f << ", board " << b << " (slot " << fboard_id_list[b] << ") ] waveform"; | ||
| TH1D* hist = tfs->make<TH1D>(hist_name.str().c_str(), hist_name.str().c_str(), wvfms[ch].size(), TTT_ini_us, TTT_end_us); |
There was a problem hiding this comment.
Do these have to be TH1D's since they are filled with values from uint16_t's? D is a double-precision floating-point number, or 64 bits per bin. You could even use TH1I. TH1S may be too short, as you have unsigned 16-bit ints as input, and they might overflow a signed 16-bit int. But 64-bit floats are way overkill.
There was a problem hiding this comment.
Hi @tomjunk thank you so much for your comments! I have pushed 3 new commits: one for the N debug waveforms to be stored, another one for fixing the type of the TH1 object and a last one for reading and processing straight out the fragments.
|
@bear-is-asleep @tomjunk Sure! I am on it, I'll let you know as soon as I update the decoder with the new suggestions. Thank you for the comments! |
…e code updated. uint type changed for unsigned int -standard-
…. Adds default values for FHiCL parameters in the constructor
|
Regarding the |
tomjunk
left a comment
tomjunk
left a comment
There was a problem hiding this comment.
Looks good! My comments have been addressed -- looks like the memory usage should be down by a lot, as it only copies one fragment at a time instead of them all, and the default is not to make a huge histogram output file in addition to the artROOT file, but you can get it if you want it. Very good!
|
@aliciavr please would you be able to resolve the conflicts? Thanks!! :) |
|
Hi @RachelCoackley, conflicts resolved :) |
|
trigger build RachelCoackley/larreco>larreco@v09_93_01rachel LArSoft/lar*@LARSOFT_SUITE_v09_93_01 SBNSoftware/sbncode@v09_93_01_p02 SBNSoftware/sbnanaobj@bug/coackley_ciCafv09_91_02 SBNSoftware/sbndaq-artdaq-core@v1_10_03 SBNSoftware/sbnobj@v09_19_05 |
|
✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard |
|
✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard |
|
❌ CI build for SBND Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard 🚨 For more details about the failed phase, check the build SBND phase logs parent CI build details are available through the CI dashboard |
|
🚨 For more details about the warning phase, check the ci_tests SBND phase logs parent CI build details are available through the CI dashboard |
|
Approved |
|
No product size differences to confirm, ci_test warnings for nucosmics_caf_quick_test_sbndcode and single_caf_quick_test_sbndcode which can be ignored (they aren't related to this PR) |
5 participants
Description
Adds a first full version of the X-ARAPUCAs decoder (V1740B digitizers).
art::EDProducer, a vector ofraw::OpDetWaveforms, are dumped.More features may be added in the future:
Checklist
Reviewers,AssigneesDevelopementRelevant PR links (optional)
Does this PR require merging another PR in a different repository (such as sbnanobj/sbnobj etc.)? No
Link(s) to docdb describing changes (optional)